Repository navigation
feat(bundler-plugins): Delete source maps with deleteSourcemapsAfterUpload: true when no upload runs - #26281
Merged
Conversation
…Upload: true` when no upload runs
Contributor
size-limit report 📦Compared: fd408c6 (PR test merge using develop at ed74679) Against: ed74679 (develop) · base run #66013 ✅ Baseline matches: this diff contains only this PR's changes.
|
timfish
requested review from
isaacs
and removed request for
stephanie-anderson
November 11, 2026 11:11
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 4 not successful, 56 successful. See the run.
Successful checks
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…th `disable-upload`
…`deleteSourcemapsAfterUpload` SDK packages turn on hidden source maps themselves, so the plugin read them as maps the user enabled and removed only the maps of uploaded chunks. Maps of chunks that `sourcemaps.assets` or `sourcemaps.ignore` leave out, and CSS maps, stayed in the build output. The internal option `sourcemaps.sdkEnabledSourceMaps` (a boolean or a function the plugin calls when it decides) tells the plugin that the SDK turned the maps on. With `deleteSourcemapsAfterUpload: true` the plugin then removes them like maps it turned on itself: all JS and CSS maps of the build, with or without an upload. Webpack, Rollup, Rolldown, Vite and esbuild read it.
…e SDK turned source maps on With `deleteSourcemapsAfterUpload: true` and `sdkEnabledSourceMaps`, the Vite plugin now also removes the maps of an output that sets `sourcemap` itself, visible ones included. The SDKs' old globs removed these maps when `build.sourcemap` was unset, so the SDKs keep that behaviour.
This was referenced Nov 13, 2026
Open
…keep maps in development With removeAllSourceMaps, Rollup and Vite now also drop maps that other plugins emit as assets, such as Vite worker bundle maps, but only when the SDK turned the maps on. With an unset setting, the plugin keeps asset maps, as it did before. In development mode the plugin no longer changes or deletes source maps, and esbuild logs again why no upload ran.
JPeer264
reviewed
Nov 13, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
Tbh I like the feature, as it makes it safer that sourcemaps are not leaked by accident. I still added one concern about a potential misconfiguration.
| plugins: [ | ||
| sentryEsbuildPlugin({ | ||
| telemetry: false, | ||
| sourcemaps: { disable: 'disable-upload', deleteSourcemapsAfterUpload: true }, |
Member
There was a problem hiding this comment.
m: That is a weird combination tbh. In this case I actually think that it would be better not to delete the sourcemaps. The true becomes more of a "force" now. As usually we recommend users to set disable-upload if they want to upload the sourcemaps themselves, but now we just delete the sourcemaps. Keeping the warning like before would still be better IMO
Collaborator
Author
There was a problem hiding this comment.
It's a weird combo to test but With disable-upload and deleteSourcemapsAfterUpload not set or set to false, sourcemaps will be kept so we're not breaking exiting configs and deleteSourcemapsAfterUpload still does what users would expect.
I think ideally for v12 we should try and consolidate/improve all the options for sourcemaps because there are so many overlapping options and they are not that clear. I think we should have multiple clearer options here rather than controlling everything via the disable property. Maybe { upload: boolean, debugIds: boolean, deleteSourcemapsAfterUpload: boolean }? And they all default to true?
Member
There was a problem hiding this comment.
Fair enough.
Could you add a todo(v12) comment so we won't forget? Definitely makes sense to make it clearer here in v12
…aps.disable` is set Users set `sourcemaps.disable` to upload the maps themselves, so `deleteSourcemapsAfterUpload: true` now keeps those maps and warns, as before. Maps an SDK turned on (`sdkEnabledSourceMaps`) are still removed, as the SDK globs did.
Co-authored-by: Jan Peer Stöcklmair <jan.peer@sentry.io>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 258f438. Configure here.
| expect(decision.log?.message).toMatch(/sourcemaps\.disable/); | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Test loop should use it.each
Low Severity
This new test loops over hidden and visible inside one it.each. The testing conventions in the review rules file ask for a separate (it)|(test).each case per scenario instead of a loop inside a single test. Flagged because that file calls this out explicitly.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 258f438. Configure here.
JPeer264
approved these changes
Nov 13, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
LGTM. One "todo comment" is still encouraged though (given in the comments) - follow up ticket works too for me.
… into separate options
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
deleteSourcemapsAfterUpload: truenow removes source maps even when no upload runs, for example without an auth token, org or project. The framework SDKs turn on hidden source maps and delete them with globs that run whether or not an upload happened. Once they move todeleteSourcemapsAfterUpload: true, a build without an auth token would otherwise keep those maps in its output. The plugin still does not turn source maps on when it cannot upload them, and in development mode (NODE_ENV=development) it changes nothing, as before. Withsourcemaps.disableset, maps the user enabled stay for a manual upload and the plugin warns, as before. The option is not released yet.The SDKs also need two internal options, on internal option types only; the public options do not change. First,
deleteSourcemapsAfterUploadcan be a function that the plugin calls when it decides, because the SDK Vite plugins only know that they turned maps on in their ownconfighook, after they created this plugin. On Vite it receives the environment name when an environment sets its ownbuild.sourcemap. Second,sdkEnabledSourceMapstells the plugin that the SDK turned the maps on. The plugin then removes every map of that build, as the SDK globs did: CSS maps, maps of chunks thatassetsorignoreleave out, maps of outputs with their ownsourcemapsetting, and maps other plugins emit as assets, such as Vite worker bundles. It also removes them withsourcemaps.disable, after it stamps the debug IDs. Without the flag, the plugin removes all maps only when it turned them on itself.